Fix checkptr in Mem.Get() on darwin#77
Conversation
Lets the tests run through on MacOS ARM by properly using Sysctl. Credit goes to elastic@2c63039
cf-foundation-community-automation
bot
moved this to Inbox
in Foundational Infrastructure Working Group
rkoster
requested review from
a team,
julian-hj and
ystros
and removed request for
a team
rkoster
moved this from Inbox
to Pending Review | Discussion
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
@jorbaum - thank you for taking a stab it getting this working on macOS!
Seems like the windows build is failing - I re-ran it just in case there was a flake and still got a failure.
It would be nice to enable macos-latest in the test workflow:
github-project-automation
bot
moved this from Pending Review | Discussion
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
Did so. |
|
Regarding the failing windows build:
However, I would not see this as part of this Pull Request. Also, it is difficult to test for me as I do not have a windows development machine. |
|
@jorbaum - it looks like the main branch is currently succeeding on Windows[1] so it seems like there is something in this PR which is causing the windows issue[2]. Perhaps you can use this PR (or another) to bring over the Win32 changes, and rely on the github worker tests to verify? Thanks, -aram [1] https://github.com/cloudfoundry/gosigar/actions/runs/17929963955/job/50984988021 |
This PR combines two critical fixes: 1. macOS Fix (from cloudfoundry#77): - Updates sigar_darwin.go to use unix.SysctlUint64() for reading sysctl values - Fixes checkptr violations in Go 1.22+ when accessing uint64 values 2. Windows Fix (from elastic#116): - Implements ProcArgs.Get() using Win32 API instead of returning ErrNotImplemented - Replaces WMI queries with direct ReadProcessMemory and NtQueryInformationProcess calls - Adds helper functions: GetUserProcessParams, ReadProcessUnicodeString, ByteSliceToStringSlice - Adds syscall bindings for ReadProcessMemory and GetTickCount64 Fixes rkoster/rubionic-workspace#38
Thx. I did not see that it worked two weeks ago. AFAICS:
I guess that this is the issue. This PR should not change any windows-related code as it only changes the darwin file.
Looks like @rubionic is already taking on this task. Thanks! @aramprice As a quick fix I could change |
...until -latest is supported again.
Did so. |
github-project-automation
bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
github-project-automation
bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
|
@jorbaum thank you for this contribution! |
3 participants
Lets the tests run through on MacOS ARM by properly using Sysctl.
Credit goes to
elastic@2c63039